Defense-in-depth: webview/path/parser hardening and Skill Finder model switch - #94
Merged
Sanjay Singh (san360) merged 6 commits intoJun 2, 2026
Merged
Conversation
…l switch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…expression' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the extension’s security posture around webview messaging, filesystem writes, parsing untrusted log input, regex safety checks, and network fetch limits, while also switching the Skill Finder’s preferred LLM family and improving diagnostics for LLM failures.
Changes:
- Hardened webview RPC boundary (message shape validation, command allowlisting, safer external URL opening).
- Added defense-in-depth for filesystem writes and network fetches (safe path joins, containment checks, redirect blocking, byte caps).
- Improved robustness against prototype pollution and ReDoS; added/expanded unit coverage for the new hardening utilities.
Show a summary per file
| File | Description |
|---|---|
| src/webview/panel.ts | Routes openExternal through URL validation and safer response handling. |
| src/webview/panel-sidebar.ts | Restricts sidebar-triggered commands to an allowlist. |
| src/webview/panel-shared.ts | Tightens request envelope validation; adds isSafeExternalHttpsUrl and safeJoinUnder. |
| src/webview/panel-shared.test.ts | Adds unit tests for new shared validators/helpers. |
| src/webview/panel-rpc.ts | Adds rule-write path containment checks and safer RPC handler lookup. |
| src/webview/panel-request-service.ts | Applies safe path joining, redirect blocking, fetch size caps, and input validation for install/catalog/GitHub flows. |
| src/webview/panel-llm.ts | Switches model family preference, improves JSON repair, and adds runtime diagnostics for LLM failures. |
| src/webview/panel-catalog.ts | Adds redirect blocking and a byte cap when reading catalog HTML pages. |
| src/webview/fetch-utils.ts | Introduces a reusable “read with byte limit” helper. |
| src/webview/fetch-utils.test.ts | Adds tests for the byte-limit reader utility. |
| src/core/parser-vscode-files.ts | Adds prototype-pollution key guards and safer own-property checks for path walking. |
| src/core/parser-vscode-files.test.ts | Adds regression tests for prototype pollution via parsed JSONL paths. |
| src/core/dsl/safe-regex.ts | Enhances alternation overlap detection for ReDoS defense. |
| src/core/dsl/safe-regex.test.ts | Expands tests for alternation overlap detection behavior. |
| .vscodeignore | Excludes dev-only tooling and marketing assets from the VSIX. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 15/15 changed files
- Comments generated: 2
…expression' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The pattern is a test fixture verifying compileSafe rejects unsafe regexes. compileSafe runs isLikelySafe() first and returns null before constructing any RegExp, so the backtracking pattern is never executed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sitive CodeQL taint-tracks the literal pattern into compileSafe's internal RegExp and cannot see the isLikelySafe() gate that rejects it first. Constructing the overlapping pattern at runtime keeps the test behavior identical while removing the analyzable regex literal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sanjay Singh (san360)
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Defense-in-depth hardening of the extension's webview message surface, parser, and supply chain, derived from a 10-point security review and a local CodeQL scan. These changes reduce the blast radius if a webview, a parsed log file, or a remote catalog response were ever malicious.
Security hardening
Webview message boundary
openExternalnow validates the URL throughisSafeExternalHttpsUrl(HTTPS + host allowed) beforevscode.env.openExternal, instead of opening any user-supplied URI/scheme. (panel.ts,panel-shared.ts)executeCommandis gated to an allowlist (aiEngineerCoach.open/.reload). (panel-sidebar.ts)isRequestMessage: shape oftype/id/method/params), and unknown methods are rejected. (panel-shared.ts,panel.ts)Filesystem writes (path-traversal defense-in-depth)
saveRuleresolves the target path and refuses to write outside the personal/project rules directories (classifyRuleWritePath/isPathUnder), covering both the new-file and existing-sourceFilePathbranches. Auto-trust on save is scoped to personal rules only — project rules stay in the trust-on-first-use review flow. (panel-rpc.ts)safeJoinUnder: per-segment^[A-Za-z0-9._-]+$validation (rejects..,\,C:\, NTFS streams, leading//), a.mdextension allowlist, and resolved-containment under~/.agents/.... Parent dirs are created before write. (panel-request-service.ts,panel-shared.ts)Parser
__proto__/constructor/prototypekeys are rejected and own-property checks replaceinwhen walking JSON paths from parsed log files. (parser-vscode-files.ts)ReDoS
^([a-z]|_)+$) are no longer wrongly rejected. (safe-regex.ts)Network / resource limits
raw.githubusercontent.comunder/github/awesome-copilot/and read through a 1 MB byte cap, preventing unbounded memory from a large/hostile response. (panel-catalog.ts,fetch-utils.ts,panel-request-service.ts)Packaging
.vscodeignore)Skill Finder fix
gpt-4.1were returning an empty stream due toinsufficient_quota; empty text surfaced as "LLM returned invalid JSON after 3 attempts." Switched the preferred LLM family togpt-5.4-miniwith a mini-first fallback chain. (panel-llm.ts)panel-llm | call-faileddiagnostic line (model id, response length, error, preview) to the runtime log so future LLM failures are debuggable.Tests
New/expanded unit tests:
safe-regex.test.ts,panel-shared.test.ts,parser-vscode-files.test.ts,fetch-utils.test.ts.Validation
npm run check— typecheck, lint, build clean; 1059/1059 tests pass.javascript-security-extendedsuite (104 queries), DB built fromsrc/— 183 TS files scanned, 0 query failures, no actionable findings (only benign warnings/false-positives).